Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

trustroot: initial client config messages #277

Merged
merged 9 commits into from
Apr 2, 2024
Merged

trustroot: initial client config messages #277

merged 9 commits into from
Apr 2, 2024

Conversation

woodruffw
Copy link
Member

@woodruffw woodruffw commented Mar 26, 2024 •
edited
Loading

WIP; needs workshopping 🙂. Closes #259.

CC @haydentherapper @loosebazooka @kommendorkapten

@woodruffw
trustroot: initial client config messages
abf547d 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw woodruffw self-assigned this Mar 26, 2024
@woodruffw
trustroot: docs
b02210e 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
codegen
3e375c5 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw woodruffw mentioned this pull request Mar 26, 2024
Closed
@woodruffw
add TSA url, rename
32b066c 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
tweak tsa_url
d7222cd 
Signed-off-by: William Woodruff <william@trailofbits.com>
haydentherapper
haydentherapper reviewed Mar 26, 2024
View reviewed changes
protos/sigstore_trustroot.proto Show resolved Hide resolved
protos/sigstore_trustroot.proto Show resolved Hide resolved
protos/sigstore_trustroot.proto Outdated Show resolved Hide resolved
@woodruffw
gen, protos: multiple tlogs and tsas
1478df1 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw @kommendorkapten
Apply suggestions from code review
9736b52 
Co-authored-by: Fredrik Skogman <kommendorkapten@github.com>
Signed-off-by: William Woodruff <william@yossarian.net>
@woodruffw woodruffw mentioned this pull request Mar 27, 2024
Closed
@haydentherapper
Copy link
Collaborator

LGTM, I think this just needs a regeneration

@woodruffw
gen: regenerate
354b78a 
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
Copy link
Member Author

LGTM, I think this just needs a regeneration

Done!

@loosebazooka loosebazooka mentioned this pull request Mar 29, 2024
Merged
@haydentherapper
Copy link
Collaborator

@woodruffw Do you want to get this in before 0.3.1?

@woodruffw
Copy link
Member Author

@haydentherapper Yep! I'll rework on top of #279 now.

@woodruffw
Merge remote-tracking branch 'upstream/main' into ww/client-trust-config
7bb5b75 
Signed-off-by: William Woodruff <william@trailofbits.com>
@haydentherapper haydentherapper mentioned this pull request Apr 1, 2024
Merged
kommendorkapten
kommendorkapten approved these changes Apr 2, 2024
View reviewed changes
Copy link
Member

@kommendorkapten kommendorkapten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Before merging, I would just like to make sure we are agreeing on how this will be distributed via PGI TUF root (i.e how this should be documented in the Sigstore Client Spec).

We now have:

  • SigningConfig
  • TrustedRoot

And then ClientTrustConfig = {SigningConfig, TrustedRoot}

The TUF repository already ships TrustedRoot as single artifact, trusted_root.json.
How would we ship the SigningConfig? My recommendation is to add a new artifact to the TUF root called signing_config.json. Why? Do not break the clients. Adding a new target is trivial.

The alternative is to add a new target client_trust_config that contains both messages. The downside is that we would need to duplicate the trusted_root.

Shipping two files from the TUF repo is not the end of the world IMHO. We can still allow clients to accept a single file (client_trust_config) to simplify the UX when e.g. invoked via the terminal.

As we are in the process of adding support for trusted root to a lot of components, withdrawing that from the TUF repository would be very disruptive 😅

@woodruffw
Copy link
Member Author

woodruffw commented Apr 2, 2024 •
edited
Loading

This might be a minority opinion, but IMO it's okay for the PGI TUF repo to not ship SigningConfig at all -- the PGI and staging instances both have "well-known" signing configurations already, so a client can (continue to) hardcode them.

In other words, IMO it's okay if SigningConfig and ClientTrustConfig exist entirely as message definitions for checking user (i.e. non-TUF) JSON inputs against, with the idea being that they establish the interface for clients that want to support CLIs like --client-trust-config trust.json.

Regardless of the above, I 100% agree about not removing the current trusted_root.json target -- IMO if we want to put SigningConfig or ClientTrustConfig in the TUF repo, then a separate file (either just the signing config or the combined client trust) would be perfectly cromulent 🙂

@kommendorkapten
Copy link
Member

Sounds good, I think we are in agreement!

@woodruffw
Copy link
Member Author

Merging!

@woodruffw woodruffw merged commit 58ba3ec into sigstore:main Apr 2, 2024
24 checks passed
@woodruffw woodruffw deleted the ww/client-trust-config branch April 2, 2024 14:52
@haydentherapper
Copy link
Collaborator

@woodruffw @kommendorkapten I think a primary motivation would be if we move Rekor to yearly sharding, since the URL would change periodically and clients would need to discover it without breaking old clients. I don't think we need to immediately ship a SigningConfig though since this isn't the case currently, but I do think we should start making plans for adding an additional file distributed through TUF.

@kommendorkapten
Copy link
Member

@haydentherapper yes, I proposed in the client meeting to today that we add the signing_config.json to the TUF repo during next signing.

@haydentherapper haydentherapper mentioned this pull request Apr 4, 2024
Open
kommendorkapten added a commit to sigstore/root-signing-staging that referenced this pull request Aug 21, 2024
@kommendorkapten
Remove github tsa from the trusted root.
6533dfb 
Added `signing_config.json` (see
sigstore/protobuf-specs#277 for details on
the name of the file).

Signed-off-by: Fredrik Skogman <kommendorkapten@github.com>
@kommendorkapten kommendorkapten mentioned this pull request Aug 21, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kommendorkapten kommendorkapten kommendorkapten approved these changes

@haydentherapper haydentherapper haydentherapper approved these changes

@loosebazooka loosebazooka Awaiting requested review from loosebazooka

Assignees

@woodruffw woodruffw

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add infrastructure information to trusted root
4 participants
@woodruffw @haydentherapper @kommendorkapten @loosebazooka